Skip to content

Conversation

@cyyever
Copy link
Contributor

@cyyever cyyever commented Dec 9, 2025

Description

This PR replaces some C-style casts with C++ ones.

Suggested changelog entry:

  • Many C-style casts were replaced with C++-style casts.

@rwgk
Copy link
Collaborator

rwgk commented Dec 13, 2025

I used Cursor like this:


Prompt

Could you please look at git diff master?

This is pybind11 PR #5930

The changes look great to me. Could you please review them to see if the C++-style casts are the best choices in all cases?

(I forgot to add: GHA testing passes, except for known issues)

Review

Overall

All of the C++-style casts introduced in this diff look appropriate, and I don’t see any cases where a different cast (static_cast, const_cast, dynamic_cast) would be safer or clearer given the CPython C-API patterns. The changes are mostly mechanical replacements of C‑style casts, with reinterpret_cast used for Python C-API struct-pointer conversions and static_cast used for numeric and void * conversions, which is exactly what you want.

Per-file notes (brief)

  • attr.h: reinterpret_cast<PyObject *>(base_info->type) is correct: PyTypeObject * and PyObject * are unrelated C structs in C++, so reinterpret_cast is the only “honest” cast. The static_cast<size_t>(has_args / has_kwargs) is fine (even optional, but explicit is clearer).

  • buffer_info.h: static_cast<ssize_t>(shape.size()/strides.size()) and static_cast<size_t>(ndim) are the right direction of cast and just make the previous intent explicit; same for static_cast<size_t>(b.itemsize).

  • cast.h: Passing reinterpret_cast<PyTypeObject *>(type::handle_of(h).ptr()) into all_type_info is appropriate; there is no safer cast for that C-API struct conversion. The static_cast<size_t>(PYBIND11_BYTES_SIZE(...)) is the right numeric cast (it was already doing that conceptually with a C-style cast).

  • detail/argument_vector.h: Using static_cast<std::size_t>(-1) and static_cast<std::size_t>(1) << bit is the correct typed way to express the existing “all bits set” / “bit mask” idioms.

  • detail/class.h: All the conversions between PyObject *, PyTypeObject *, and PyHeapTypeObject * now use reinterpret_cast, which is exactly what’s needed for CPython’s “struct inheritance” in C++; static_cast would not be valid here. static_cast<char *>(PyMem_MALLOC(size)) is the right choice for void *char *.

  • detail/cpp_conduit.h: The bool-casts were redundant; returning the boolean expression directly is fine and keeps the function signatures clear.

  • detail/function_record_pyobject.h: reinterpret_cast<detail::function_record_PyObject *>(obj) and reinterpret_steal<object>(reinterpret_cast<PyObject *>(py_func_rec)) are appropriate: these are known layout reinterprets for a custom Python heap type.

  • detail/init.h: Casting v_h.inst to PyObject * via reinterpret_cast when interacting with getattr/setattr is correct: instance is a CPython heap object struct, so again this is the right kind of cast.

  • detail/internals.h: Just removing the redundant null-check before delete; no cast issues here.

  • detail/type_caster_base.h: All new reinterpret_cast<PyTypeObject *> and reinterpret_cast<PyObject *> uses are in the same pattern as elsewhere and are necessary. static_cast<void **>(PyMem_Calloc(...)) is the right cast from void * for an allocation.

  • detail/value_and_holder.h: The bitmask updates now use static_cast<std::uint8_t>(~instance::status_...); this is a faithful, type-safe replacement for the old C-style casts and still correctly truncates the complemented mask to 8 bits.

  • pybind11.h: Pointer casts between PyObject *, PyTypeObject *, PyHeapTypeObject *, and the various internal structs (function_record_PyObject, PyCFunctionObject, instance) are all correctly expressed with reinterpret_cast. static_cast<std::uint16_t>(args) and static_cast<size_t>(PyTuple_GET_SIZE(...)) accurately document the narrowing/widening already happening.

  • pytypes.h: handle(reinterpret_cast<PyObject *>(Py_TYPE(h.ptr()))) is the right way to turn a PyTypeObject * into a handle. All the size/length conversions now using static_cast<size_t> and the float conversions using static_cast<double> / static_cast<float> are straightforward and correct; the as_unsigned changes preserve the established “(Unsigned)-1 sentinel” behaviour.

Bottom line

I don’t see any misused C++-style casts in this PR; the choices between reinterpret_cast and static_cast are consistent with CPython’s C-API conventions and with pybind11’s existing style, and there are no places where a safer cast type is obviously available. Any further tweaks would be purely stylistic, not correctness or safety fixes.

Copy link
Collaborator

@rwgk rwgk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks for doing this.

@rwgk rwgk merged commit 032e73d into pybind:master Dec 13, 2025
81 of 85 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Dec 13, 2025
@cyyever cyyever deleted the c_cast branch December 13, 2025 06:15
@rwgk rwgk removed the needs changelog Possibly needs a changelog entry label Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants